Fixing learning hub hero#1360
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Learning Hub homepage hero markup and global styles to adjust spacing/whitespace behavior in the hero area.
Changes:
- Adds a global
.main-framepadding override. - Removes
.heropadding rules (including the mobile breakpoint variant). - Simplifies the hero title markup from two gradient spans to one, along with formatting-only changes in
index.astro.
Show a summary per file
| File | Description |
|---|---|
| website/src/styles/global.css | Introduces .main-frame padding override and removes hero padding rules affecting layout spacing. |
| website/src/pages/index.astro | Updates hero title markup (single gradient span) and reformats hero/search markup. |
Copilot's findings
Comments suppressed due to low confidence (2)
website/src/styles/global.css:210
- The
.herosection padding was removed entirely. Since no other.heropadding rule exists in the codebase, this will change the vertical spacing of the homepage hero (and any other.herosections) rather than addressing word/whitespace between title spans. Consider restoring the hero padding and fixing the title spacing in a more targeted way (e.g., adjusting spacing on the title spans).
This issue also appears on line 1881 of the same file.
/* Hero Section */
.hero {
text-align: center;
position: relative;
background: transparent;
}
website/src/styles/global.css:1884
- The mobile
.heropadding override inside the@media (max-width: 768px)block was removed. Combined with the base.heropadding removal, the hero now has no explicit vertical padding on small screens, which is likely to regress layout/spacing. Consider reinstating a mobile-specific hero padding rule (or otherwise ensuring consistent hero spacing across breakpoints).
@media (max-width: 768px) {
.hero-particle {
opacity: 0.2;
}
- Files reviewed: 2/2 changed files
- Comments generated: 3
| padding: 0 24px; | ||
| } | ||
|
|
||
| .main-frame { |
There was a problem hiding this comment.
.main-frame is introduced here, but there are no other references to main-frame in the repo sources (only this CSS rule). If this is meant to override a Starlight/third-party layout class, it should be scoped (e.g., under body:has(#main-content) / #main-content) and/or documented; otherwise consider removing it to avoid a global, hard-to-trace layout override.
| .main-frame { | |
| body:has(#main-content) .main-frame { |
| <h1 id="hero-heading"> | ||
| <span class="gradient-text">Awesome</span> | ||
| <span class="gradient-text-alt">GitHub Copilot</span> | ||
| <span class="gradient-text">Awesome GitHub Copilot</span> |
There was a problem hiding this comment.
The hero heading was changed from two spans (gradient-text + gradient-text-alt) to a single gradient-text span. This changes the intended visual design (single gradient instead of two) and leaves the .gradient-text-alt styling unused; if the goal is just to fix the extra gap between “Awesome” and “GitHub Copilot”, it would be more targeted to keep the two spans and adjust the spacing/margin on the second span instead.
| <span class="gradient-text">Awesome GitHub Copilot</span> | |
| <span class="gradient-text">Awesome</span> | |
| <span class="gradient-text-alt">GitHub Copilot</span> |
| <section class="hero" aria-labelledby="hero-heading"> | ||
| <div class="container"> | ||
| <h1 id="hero-heading"> | ||
| <span class="gradient-text">Awesome</span> | ||
| <span class="gradient-text-alt">GitHub Copilot</span> | ||
| <span class="gradient-text">Awesome GitHub Copilot</span> | ||
| </h1> |
There was a problem hiding this comment.
PR description mentions only changing .gradient-text-alt margin-left, but the implementation instead removes the .gradient-text-alt span and also changes global hero/layout spacing. Please update the PR description to match the actual approach, or adjust the changes to align with the stated intent (a targeted word-spacing fix).
Tweaking how the whitespace is used.
Replaces #1357